Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add resource type #189

Merged
merged 8 commits into from
Sep 14, 2023
Merged

feat: add resource type #189

merged 8 commits into from
Sep 14, 2023

Conversation

JonasKruckenberg
Copy link
Member

@JonasKruckenberg JonasKruckenberg commented Sep 10, 2023 •
edited
Loading

This finally fixes the implementation of the resource type.

Details

Take the following wit file:

interface has_resource {
   func constructor_a () -> a

   resource a {
      func f1()
   }
}

It translates into this host code:

use tauri_bindgen_host::{ResourceTable, ResourceId};

// resources are represented by their own structs, allowing them to keep their own state.
struct ResourceA;

impl has_resource::A for ResourceA {
    fn f1(&self) {
        println!("called resource method!!")
    }
}

struct Ctx {
    // there is no centrally managed resource table as with wasmtime or deno
    // instead callers manage their own resource tables.
    // While this is more verbose it's both much simpler and reduces lock contention (by splitting one big lock into multiple small ones)
    a: ResourceTable,
}

impl has_resource::HasResource for Ctx {
    type A = ResourceA;

    // this is required under the hood so the dispatcher can get the resource
    fn get_a(&self, id: ResourceId) -> ::tauri_bindgen_host::Result<Arc<Self::A>> {
        self.a.get::<Self::A>(id)
    }

    // this is the regular interface function
    fn construct_a(&self) -> ResourceId {
        self.a.push(Arc::new(ResourceA)).unwrap()
    }
}

TODO list

  • Host implementation
  • Guest JS implementation
  • Guest TS implementation
  • Guest Rust implementation
  • Markdown implementation

@netlify
Copy link

netlify bot commented Sep 10, 2023 •
edited
Loading

✅ Deploy Preview for tauri-bindgen canceled.

Name Link
🔨 Latest commit dd85e41
🔍 Latest deploy log https://app.netlify.com/sites/tauri-bindgen/deploys/6502d90fd591e600086ab0a4

@JonasKruckenberg JonasKruckenberg requested review from lucasfernog and removed request for lucasfernog September 10, 2023 12:41
@JonasKruckenberg
Copy link
Member Author

Requested a review from @tauri-apps/wg-webview bc you are probably the most appropriate for this field, not really sure though

@amrbashir
Copy link
Member

amrbashir commented Sep 10, 2023 •
edited
Loading

I think @tauri-apps/wg-tauri is better suited for this, however I have not been following the tauri-bindgen closely so I am not sure what I should be looking at to review.

I should not that I needed to have a resource table in tauri-apps/tauri#7709 so I used deno's implementation (but with Arc instead of Rc because of tauri's Send + Sync requirements) so this PR would replace that implementation, right?

struct Ctx {
    // there is no centrally managed resource table as with wasmtime or deno
    // instead callers manage their own resource tables.
    // While this is more verbose it's both much simpler and reduces lock contention (by splitting one big lock into multiple small ones)
    a: ResourceTable,
}

How would this struct look like if I have multiple resources? would it look like this?

struct Ctx {
    a: ResourceTable,
    b: ResourceTable,
    c: ResourceTable,
}

If so, it seems unnecessary, if applying to the menu PR I linked above, where I usually need to use 2~4 different menu types (aka resources), I will have to lock 2~4, instead of just one lock with a central resource table.

@JonasKruckenberg
Copy link
Member Author

Yeah so I slapped this ResourceTable impl together just to have something that works. But you're not forced to use it at all, the idea is that the generated trait forces you to implement a method called get_<resource ident>(&self, id: ResourceId) and that will be used under the hook to determine which resource's mehthods will be called.

It doesn't really matter what structure you use to store the resources as long as you can produce a reference to it when asked.

@JonasKruckenberg JonasKruckenberg linked an issue Sep 12, 2023 that may be closed by this pull request
[feat] implement resource #44
Closed
@JonasKruckenberg JonasKruckenberg merged commit 4fc443e into main Sep 14, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucasfernog lucasfernog lucasfernog approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[feat] implement resource
3 participants
@JonasKruckenberg @amrbashir @lucasfernog